console_linux: Fix race: lock Cond before Signal. #27
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
It looks, like your travis install/config has some problems. |
|
Thanks, looking into the travis issue |
|
@gerasiov could you rebase this on master? We merge a travis fix for the failing CI. |
Possible race: Thread1: Thread2: line 178: Read() -> EAGAIN ... <reschedule> line 110: EpollWait() ... line 124: signalRead() ... ... line 191: Wait() ... line 110: EpollWait() Thread2 (epoll) sends signalRead() (via sync.Cond) but Thread1 is not waiting yet. Then it starts to wait, but no more signals will arrive (because Thread2 is sleeping in EpollWait() on edge-triggered epoll. To prevent this race one should held Lock when sending signalRead(). The same situation goes with Write loop. Bug was reported to docker: docker/for-linux#353 Signed-off-by: Alexander Gerasiov <gerasiov@yandex-team.ru>
|
LGTM |
mikebrow
reviewed
Jul 5, 2018
| ec.readc.Signal() | ||
| ec.readc.L.Unlock() |
There was a problem hiding this comment.
nit.. maybe use defer for the Unlock() calls to mirror existing code pattern.
This was referenced Jul 6, 2018
|
Is there a way to track when this would make it into a Docker release? |
|
@djmitche it looks like 18.06 has the fix: moby/moby#35865 (comment) |
petrosagg
added a commit
to balena-os/balena-engine
that referenced
this pull request
Sep 21, 2018
Signed-off-by: Petros Angelatos <petrosagg@gmail.com> Connects-To: moby/moby#35865 Connects-To: containerd/console#27
thaJeztah
added a commit
to thaJeztah/runc
that referenced
this pull request
Mar 28, 2019
relevant changes: - containerd/console#27 console_linux: Fix race: lock Cond before Signal Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
thaJeztah
added a commit
to thaJeztah/runc
that referenced
this pull request
Mar 28, 2019
relevant changes: - containerd/console#27 console_linux: Fix race: lock Cond before Signal Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
thaJeztah
added a commit
to thaJeztah/runc
that referenced
this pull request
Apr 12, 2019
relevant changes: - containerd/console#27 console_linux: Fix race: lock Cond before Signal Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
thaJeztah
added a commit
to thaJeztah/runc
that referenced
this pull request
Apr 12, 2019
relevant changes: - containerd/console#27 console_linux: Fix race: lock Cond before Signal Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
thaJeztah
added a commit
to thaJeztah/runc
that referenced
this pull request
Apr 12, 2019
relevant changes: - containerd/console#27 console_linux: Fix race: lock Cond before Signal Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
thaJeztah
added a commit
to thaJeztah/runc
that referenced
this pull request
Apr 25, 2019
relevant changes: - containerd/console#27 console_linux: Fix race: lock Cond before Signal Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
thaJeztah
added a commit
to thaJeztah/runc
that referenced
this pull request
Jul 14, 2019
relevant changes: - containerd/console#27 console_linux: Fix race: lock Cond before Signal Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
thaJeztah
added a commit
to thaJeztah/runc
that referenced
this pull request
Aug 26, 2019
relevant changes: - containerd/console#27 console_linux: Fix race: lock Cond before Signal Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
thaJeztah
added a commit
to thaJeztah/runc
that referenced
this pull request
Sep 5, 2019
relevant changes: - containerd/console#27 console_linux: Fix race: lock Cond before Signal Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
stefanberger
pushed a commit
to stefanberger/runc
that referenced
this pull request
Dec 31, 2019
relevant changes: - containerd/console#27 console_linux: Fix race: lock Cond before Signal Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
adrianreber
pushed a commit
to adrianreber/runc
that referenced
this pull request
Feb 10, 2020
relevant changes: - containerd/console#27 console_linux: Fix race: lock Cond before Signal Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Possible race:
Thread2 (epoll) sends signalRead() (via sync.Cond) but Thread1 is not waiting
yet. Then is start to wait, but no more signals will arrive (because Thread2
is sleeping in EpollWait() on edge-triggered epoll.
To prevent this race one should held Lock when sending signalRead().
The same situation goes with Write loop.
Bug was reported to docker: docker/for-linux#353
Signed-off-by: Alexander Gerasiov gerasiov@yandex-team.ru